Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

samples: matter: lock: Support all fields of LockOperation event #21078

Merged
merged 7 commits into from
Mar 24, 2025
Merged

samples: matter: lock: Support all fields of LockOperation event #21078

merged 7 commits into from
Mar 24, 2025

Conversation

adigie
Copy link
Member

@adigie adigie commented Mar 20, 2025

  • Remove redundant call to BoltLockManager::[Lock|Unlock] from
    MatterPostAttributeChangeCallback.
  • Return ValidatePINResult containing user id and credential used for
    validation.
  • Add support for UserIndex, FabricIndex, SourceNode and
    Credentials fields.

@adigie adigie requested a review from a team as a code owner March 20, 2025 19:53
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Mar 20, 2025
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Mar 20, 2025 •
edited
Loading

CI Information

To view the history of this post, clich the 'edited' button above
Build number: 9

Inputs:

Sources:

sdk-nrf: PR head: 778e7318d01ace3e016c3d32ea501a9200f2f3a6

more details

sdk-nrf:

PR head: 778e7318d01ace3e016c3d32ea501a9200f2f3a6
merge base: 55d0fe25b2e97dbf7db51c703243ded53a28d49a
target head (main): 648c06e61f46d5f2f56ff94db222762c663fd519
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (8) 
doc
│  ├── nrf
│  │  ├── releases_and_maturity
│  │  │  ├── releases
│  │  │  │  │ release-notes-changelog.rst
samples
│  ├── matter
│  │  ├── lock
│  │  │  ├── src
│  │  │  │  ├── access
│  │  │  │  │  ├── access_manager.cpp
│  │  │  │  │  │ access_manager.h
│  │  │  │  ├── app_task.cpp
│  │  │  │  ├── app_task.h
│  │  │  │  ├── bolt_lock_manager.cpp
│  │  │  │  ├── bolt_lock_manager.h
│  │  │  │  │ zcl_callbacks.cpp

Outputs:

Toolchain

Version: 4ffa2202d5
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:4ffa2202d5_e579f9fbe6

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
    • sdk-nrf test count: 97
  • ✅ Integration tests
    • ✅ test-fw-nrfconnect-chip
Disabled integration tests
    • desktop52_verification
    • doc-internal
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_samples
    • test-fw-nrfconnect-boot
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_cloud
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_mosh
    • test-fw-nrfconnect-nrf-iot_positioning
    • test-fw-nrfconnect-nrf-iot_samples
    • test-fw-nrfconnect-nrf-iot_serial_lte_modem
    • test-fw-nrfconnect-nrf-iot_thingy91
    • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • test-fw-nrfconnect-nrf_crypto
    • test-fw-nrfconnect-proprietary_esb
    • test-fw-nrfconnect-ps
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-rs
    • test-fw-nrfconnect-tfm
    • test-fw-nrfconnect-thread
    • test-fw-nrfconnect-zigbee
    • test-low-level
    • test-sdk-audio
    • test-sdk-dfu
    • test-sdk-find-my
    • test-sdk-mcuboot
    • test-sdk-pmic-samples
    • test-sdk-wifi
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

@adigie adigie force-pushed the lock-operation-event-fix branch 2 times, most recently from 73fc0e6 to 4868011 Compare March 20, 2025 21:02
ArekBalysNordic
ArekBalysNordic reviewed Mar 21, 2025
View reviewed changes
Copy link
Contributor

@ArekBalysNordic ArekBalysNordic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've changed the door lock API, so I think at least a release or migration guide note should be added.

samples/matter/lock/src/access/access_manager.h
* @return true on success, false otherwise.
*/
bool ValidatePIN(const Optional<chip::ByteSpan> &pinCode, OperationErrorEnum &err);
bool ValidatePIN(const Optional<chip::ByteSpan> &pinCode, OperationErrorEnum &err,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markaj-nordic just FYI, the doorlock access manager API has been changed.

@adigie adigie force-pushed the lock-operation-event-fix branch 2 times, most recently from 9d32699 to d02c1ab Compare March 21, 2025 09:25
adigie added 2 commits March 21, 2025 10:51
@adigie
samples: matter: lock: Format sources
4782d4a 
Format sources before changes.

Signed-off-by: Adrian Gielniewski <adrian.gielniewski@nordicsemi.no>
@adigie
samples: matter: lock: Remove Lock/Unlock from callback
04dbb37 
Remove redundant call to `BoltLockManager::[Lock|Unlock]` from
MatterPostAttributeChangeCallback.

Signed-off-by: Adrian Gielniewski <adrian.gielniewski@nordicsemi.no>
@adigie adigie force-pushed the lock-operation-event-fix branch from d02c1ab to 47b31c8 Compare March 21, 2025 09:51
@adigie adigie requested a review from ArekBalysNordic March 21, 2025 13:37
@adigie
Copy link
Member Author

adigie commented Mar 21, 2025

Addressed SonarQube issues.

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
3.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@adigie adigie requested a review from a team March 21, 2025 16:02
kkasperczyk-no
kkasperczyk-no reviewed Mar 24, 2025
View reviewed changes
@kkasperczyk-no
Copy link
Contributor

You've changed the door lock API, so I think at least a release or migration guide note should be added.

Yeah, release note entry would not harm.

adigie added 4 commits March 24, 2025 10:42
@adigie
samples: matter: lock: Add ValidatePINResult
612adaa 
Return `ValidatePINResult` containing user id and credential used for
validation.

Signed-off-by: Adrian Gielniewski <adrian.gielniewski@nordicsemi.no>
@adigie
samples: matter: lock: Support all fields of LockOperation event
a8b61c8 
Add support for `UserIndex`, `FabricIndex`, `SourceNode` and
`Credentials` fields.

Signed-off-by: Adrian Gielniewski <adrian.gielniewski@nordicsemi.no>
@adigie
samples: matter: lock: Reduce code nesting
09e1880 
Address SonarQube warning regarding code nesting.

Signed-off-by: Adrian Gielniewski <adrian.gielniewski@nordicsemi.no>
@adigie
samples: matter: lock: Extract lambda to separate function
a174d22 
Address SonarQube issue regarding too long lambda body.

Signed-off-by: Adrian Gielniewski <adrian.gielniewski@nordicsemi.no>
@adigie adigie force-pushed the lock-operation-event-fix branch from 4ca1f29 to b98f9bb Compare March 24, 2025 10:10
@adigie adigie requested a review from a team as a code owner March 24, 2025 10:10
@github-actions github-actions bot added doc-required PR must not be merged without tech writer approval. and removed changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Mar 24, 2025
@github-actions GitHub Actions
Copy link

You can find the documentation preview for this PR here.

Preview links for modified nRF Connect SDK documents:

https://ncsdoc.z6.web.core.windows.net/PR-21078/nrf/releases_and_maturity/releases/release-notes-changelog.html

@adigie
doc: samples: matter: lock: Add changelog entry regarding API changes.
778e731 
Mention API changes in `lock` sample.

Signed-off-by: Adrian Gielniewski <adrian.gielniewski@nordicsemi.no>
@adigie adigie force-pushed the lock-operation-event-fix branch from b98f9bb to 778e731 Compare March 24, 2025 10:33
@nordicjm nordicjm merged commit cd4d017 into nrfconnect:main Mar 24, 2025
13 of 14 checks passed
@adigie adigie deleted the lock-operation-event-fix branch March 24, 2025 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peknis peknis peknis approved these changes

@kkasperczyk-no kkasperczyk-no kkasperczyk-no approved these changes

@ArekBalysNordic ArekBalysNordic ArekBalysNordic approved these changes

Assignees
No one assigned
Labels
doc-required PR must not be merged without tech writer approval.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@adigie @NordicBuilder @kkasperczyk-no @peknis @ArekBalysNordic @nordicjm